Trigger only on "READY TO REVIEW" label#18
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -123,12 +124,13 @@ runs: | |||
| PR_NUMBER="$PR_NUMBER" | |||
| CACHE_HIT="${{ steps.claudecode-history.outputs.cache-hit }}" | |||
|
|
|||
| # Check if required label is present | |||
| if [ -n "$REQUIRE_LABEL" ]; then | |||
| # Check label requirement only for 'labeled' trigger | |||
| # Other triggers (opened, synchronize, reopened) run without label check | |||
| if [ "$EVENT_ACTION" == "labeled" ] && [ -n "$REQUIRE_LABEL" ]; then | |||
| if echo "$PR_LABELS" | jq -e --arg label "$REQUIRE_LABEL" 'index($label) != null' > /dev/null 2>&1; then | |||
| echo "Required label '$REQUIRE_LABEL' found on PR #$PR_NUMBER" | |||
| echo "Required label '$REQUIRE_LABEL' found on PR #$PR_NUMBER (triggered by label addition)" | |||
| else | |||
| echo "Skipping code review: required label '$REQUIRE_LABEL' not found on PR #$PR_NUMBER" | |||
| echo "Skipping code review: 'labeled' trigger requires label '$REQUIRE_LABEL', but it was not the label added" | |||
| ENABLE_CLAUDECODE="false" | |||
| fi | |||
There was a problem hiding this comment.
🤖 Code Review Finding: Label check doesn't verify which label was added
Severity: MEDIUM
Category: correctness
Impact: When any label is added to a PR that already has the required label, the review will run even though a different label was added. The error message "it was not the label added" is misleading since the code doesn't check which label was added.
Recommendation: To check the actual label added, use github.event.label.name instead. Pass it as an environment variable (e.g., ADDED_LABEL: ${{ github.event.label.name }}) and compare it to REQUIRE_LABEL.
| ADDED_LABEL: ${{ github.event.label.name }} | |
| run: | | |
| # Check if ClaudeCode should be enabled | |
| ENABLE_CLAUDECODE="true" | |
| SILENCE_CLAUDECODE_COMMENTS="false" | |
| # For PRs, check sampling and cache | |
| if [ "${{ github.event_name }}" == "pull_request" ]; then | |
| PR_NUMBER="$PR_NUMBER" | |
| CACHE_HIT="${{ steps.claudecode-history.outputs.cache-hit }}" | |
| # Check label requirement only for 'labeled' trigger | |
| # Other triggers (opened, synchronize, reopened) run without label check | |
| if [ "$EVENT_ACTION" == "labeled" ] && [ -n "$REQUIRE_LABEL" ]; then | |
| if [ "$ADDED_LABEL" == "$REQUIRE_LABEL" ]; then | |
| echo "Required label '$REQUIRE_LABEL' added to PR #$PR_NUMBER (triggered by label addition)" | |
| else | |
| echo "Skipping code review: 'labeled' trigger requires label '$REQUIRE_LABEL', but '$ADDED_LABEL' was added" | |
| ENABLE_CLAUDECODE="false" | |
| fi | |
| fi |
|
@matej How exactly do you want the As-is, it procs every time a label is applied (regardless of what that label is). I don't think that's the behaviour we want. The way I see it, there's one of two options:
IMO option 2 is better because it better accounts for letting the stuff run on manual trigger for debugging. But also since we have the on_PR trigger happening, it also means we would trigger a code review when the PR is first opened (and maybe we want that? Thoughts?) |
|
The intention is that no review is performed at all if the label is not present. So it always needs to be check. If I open a PR and this label is not present then we're considering it a draft PR and are not requesting code review. Once the label is applied we want an immediate code review but we also want to re-review on any new commit. That is effectively how the label works in the Nutrient monorepo. |
2 participants
The current version of the workflow triggers on any label applied; we only want this to trigger on "READY TO REVIEW" items.